Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable Dilithium and SPHINCS+ sig algs by default #406

Closed
wants to merge 1 commit into from

Conversation

iyanmv
Copy link
Member

@iyanmv iyanmv commented May 1, 2024

This fixes #399

With the current default enabled sig algs, some servers may fail to complete the TLS handshake. This is probably not an OpenSSL or oqsprovider bug but a buggy TLS implementation on the server side. I guess something similar to what is described in https://tldr.fail/.

Until the issue is better understood, let's enable less sig algs by default so that the chances of users being affected by this issue are lower.

The only file manually edited was oqs-template/generate.yml with

sed -i -e 's/enable: true/enable: false/g' oqs-template/generate.yml
sed -i -e '552,660s/enable: false/enable: true/g' oqs-template/generate.yml
sed -i -e '661,763s/enable: false/enable: true/g' oqs-template/generate.yml

The rest of the files were generated with

bash oqs-template/generate.sh

@iyanmv iyanmv requested a review from baentsch as a code owner May 1, 2024 07:22
This fixes open-quantum-safe#399

With the current default enabled sig algs, some servers may fail to
complete the TLS handshake. This is probably not an OpenSSL or
oqsprovider bug but a buggy TLS implementation on the server side. I
guess something similar to what is described in https://tldr.fail/.

Until the issue is better understood, let's enable less sig algs by
default so that the changes of users being affected by this issue are
lower.

The only file manually edited was oqs-template/generate.yml with

```shell
sed -i -e 's/enable: true/enable: false/g' oqs-template/generate.yml
sed -i -e '552,660s/enable: false/enable: true/g' oqs-template/generate.yml
sed -i -e '661,763s/enable: false/enable: true/g' oqs-template/generate.yml
```

The rest of the files were generated with

```shell
bash oqs-template/generate.sh
```

Signed-off-by: Iyán Méndez Veiga <[email protected]>
Copy link
Member

@baentsch baentsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this analysis and proposal! How many sigalgs do these " buggy servers" support OK? Can we refer to a (TLS) spec doc that confirms this as truly buggy? If so, let's try to find out those servers' software and raise an issue; completely disabling by default two complete PQ sigalg families seems too strong a limitation for oqsprovider users in response. If oqsprovider in turn violates a spec (sending too many code points?) such a limitation may be called for; but I'd suggest doing a poll first what should be removed from the default set. Finally, the generate.py code generator must run with idempotence (this property is checked by CI and must be assured before merge).

@beldmit
Copy link
Contributor

beldmit commented May 2, 2024

I'm not sure it's worth disable Dilithium as for now - it may be used by some existing installations and it's worth having an option to test the compatibility

@iyanmv
Copy link
Member Author

iyanmv commented May 2, 2024

Thanks for this analysis and proposal! How many sigalgs do these " buggy servers" support OK?

These particular two servers (crates.io:443 and fwupd.org:443) that I've been using to test, fail with sig algs between 40 and 43. I can't give an exact number just playing with the generate.yml.

Can we refer to a (TLS) spec doc that confirms this as truly buggy? If so, let's try to find out those servers' software and raise an issue; completely disabling by default two complete PQ sigalg families seems too strong a limitation for oqsprovider users in response. If oqsprovider in turn violates a spec (sending too many code points?) such a limitation may be called for; but I'd suggest doing a poll first what should be removed from the default set.

I don't think there is such TLS spec limiting the number of sig algs in the Client hello message, but I will double check. I can also create an issue in fwupd repo, author is very responsive and maybe he can give some hints about why the server fails to respond correctly.

Finally, the generate.py code generator must run with idempotence (this property is checked by CI and must be assured before merge).

I tried to achieve this by using the generate.sh script, but I guess I did something wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Too many advertised sig algs cause TLS server hang-up
3 participants